Skip to content

Improve MOTW error handling in download flow#6127

Merged
JohnMcPMS merged 5 commits intomicrosoft:masterfrom
Trenly:Downloader
Apr 13, 2026
Merged

Improve MOTW error handling in download flow#6127
JohnMcPMS merged 5 commits intomicrosoft:masterfrom
Trenly:Downloader

Conversation

@Trenly
Copy link
Copy Markdown
Contributor

@Trenly Trenly commented Apr 6, 2026

Fix MOTW security check failures for downloaded installers

Problem

All installer downloads were failing the security check with 0x80070003 (ERROR_PATH_NOT_FOUND), causing every install to terminate with "Installer failed security check".

Two root causes were identified:

  • Wrong file scanned by IAttachmentExecute — UpdateInstallerFileMotwIfApplicable ran before RenameDownloadedInstaller in the workflow. This meant the Attachment Execution Service (AES) was scanning the
    temporary pre-hash-validation file (a raw SHA256 hex string with no extension, e.g. aab2dc8e…) rather than the final installer (e.g. BadlionClient.exe).

Important

Shell32's AES relies on file extension for MIME-type detection, scan policy, and zone assignment, so scanning an extensionless file produced unreliable results

  • RemoveMotwIfApplicable failures aborted the security check — If removing the existing Zone.Identifier ADS failed, the exception propagated through the AES thread, leaving IAttachmentExecute::Save() never
    called and the whole check returning a failure code. Additionally, a missing THROW_IF_FAILED(hr) after IPersistFile::Load meant unexpected load errors were silently ignored, leading to Remove()/Save()
    being called on a non-loaded object.

Changes

DownloadFlow.cpp

  • Reorder workflow: RenameDownloadedInstaller now runs before UpdateInstallerFileMotwIfApplicable, so AES always scans the properly-named installer file.

Downloader.cpp — RemoveMotwIfApplicable

  • Also handle ERROR_PATH_NOT_FOUND (in addition to ERROR_FILE_NOT_FOUND) from IPersistFile::Load — different Windows builds return different codes when no Zone.Identifier ADS is present.
  • Restore missing THROW_IF_FAILED(hr) to correctly surface any other Load failures rather than falling through to Remove()/Save() on an unloaded object.

Downloader.cpp — ApplyMotwUsingIAttachmentExecuteIfApplicable

  • Wrap RemoveMotwIfApplicable in a try/catch inside the updateMotw lambda — log a warning but proceed to IAttachmentExecute::Save(). MOTW removal is best-effort; a removal failure should not abort the
    security scan.
  • Wrap the entire AES thread body in try/catch and log exceptions via AICLI_LOG, so unhandled errors are surfaced rather than silently leaving hr in an indeterminate state. Use LOG_IF_FAILED for
    CoInitializeEx so failures are captured through WIL.
  • Fix the return value: previously aesSaveResult was always returned, reporting S_OK when Save() was never reached (e.g. after CoInitializeEx failure). Now returns the thread's hr when aesSaveResult was
    never updated

Microsoft Reviewers: Open in CodeFlow

@Trenly Trenly marked this pull request as ready for review April 6, 2026 21:25
@Trenly Trenly requested a review from a team as a code owner April 6, 2026 21:25
@Trenly Trenly changed the title Surface errors instead of swallowing Ensure MOTW failures are called out in logs Apr 6, 2026
@github-actions

This comment has been minimized.

@Dragon1573
Copy link
Copy Markdown

Sorry to ask here. 🙏🏼

Is there any unknown reason that blocks this PR? A new preview version of winget.exe was released but I still unable to install packages from local manifests ...

@JohnMcPMS
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@@ -536,21 +549,31 @@ namespace AppInstaller::Utility

std::thread aesThread([&]()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that the failures being missing from the logs is probably due to this new thread. If you want this change to actually make the logs show up and not just fix the error handling, you would need to:

// outside lambda
ThreadGlobals* globals = ThreadGlobals::GetForCurrentThread();
// capture into lambda
... = [..., globals] (...)
{
if (globals)
{
  auto globalsCleanup = globals->SetForCurrentThread();
}
...
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the error handling is the main thing causing the issue, my preference is to get this error handling merged so that some portion of the local manifests start working again for users who are having issues, and I can raise a second PR for the logging globals

@Trenly Trenly changed the title Ensure MOTW failures are called out in logs Improve MOTW error handling in download flow Apr 13, 2026
@JohnMcPMS JohnMcPMS merged commit 5c46ba1 into microsoft:master Apr 13, 2026
9 checks passed
@Trenly Trenly deleted the Downloader branch April 13, 2026 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants